<html>
<head><meta charset="utf-8"><title>Split up astconv.rs · t-compiler · Zulip Chat Archive</title></head>
<h2>Stream: <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/index.html">t-compiler</a></h2>
<h3>Topic: <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Split.20up.20astconv.2Ers.html">Split up astconv.rs</a></h3>

<hr>

<base href="https://rust-lang.zulipchat.com">

<head><link href="https://rust-lang.github.io/zulip_archive/style.css" rel="stylesheet"></head>

<a name="207256684"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Split%20up%20astconv.rs/near/207256684" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Arthur Cohen <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Split.20up.20astconv.2Ers.html#207256684">(Aug 18 2020 at 13:07)</a>:</h4>
<p>Hi people, I recently started working on <a href="https://github.com/rust-lang/rust/issues/67418">this issue</a> and am encountering a bit of an issue. The trait <code>AstConv</code> has an <code>impl dyn trait</code> that spans over 3000 lines. I'm unsure as to how to split them into one super trait and a few subtraits. Would it be much of a breaking change ? It does not seem to have too much impact, only in some places of the compiler. However, it would break some of the encapsulation introduced by the use of an <code>impl dyn trait</code>.</p>



<a name="207256922"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Split%20up%20astconv.rs/near/207256922" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Matthew Jasper <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Split.20up.20astconv.2Ers.html#207256922">(Aug 18 2020 at 13:10)</a>:</h4>
<p>The impl could be split into multiple blocks, and then across files.</p>



<a name="207257237"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Split%20up%20astconv.rs/near/207257237" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Arthur Cohen <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Split.20up.20astconv.2Ers.html#207257237">(Aug 18 2020 at 13:12)</a>:</h4>
<p><span class="user-mention silent" data-user-id="116118">Matthew Jasper</span> <a href="#narrow/stream/131828-t-compiler/topic/Split.20up.20astconv.2Ers/near/207256922">said</a>:</p>
<blockquote>
<p>The impl could be split into multiple blocks, and then across files.</p>
</blockquote>
<p>I thought about that, but it would allow users of the AstConv trait to override those methods unless I'm mistaken. And I believe (but I might be wrong) that the main advantage of <code>impl dyn trait</code> is to disallow overriding of the methods while still allowing to call them</p>



<a name="207260989"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Split%20up%20astconv.rs/near/207260989" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Matthew Jasper <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Split.20up.20astconv.2Ers.html#207260989">(Aug 18 2020 at 13:47)</a>:</h4>
<p>The new impls are still <code>impl dyn trait</code>, there's no restriction on how many you have.</p>



<a name="207514041"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Split%20up%20astconv.rs/near/207514041" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Arthur Cohen <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Split.20up.20astconv.2Ers.html#207514041">(Aug 20 2020 at 11:56)</a>:</h4>
<p>Hey <span class="user-mention" data-user-id="124288">@oli</span> , would this be a good place to discuss the reviews you left on my PR ? I'm not sure I entirely get the first message you left, as right now there's only <code>astconv/mod.rs</code> and not <code>astconv.rs</code> anymore</p>



<a name="207514057"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Split%20up%20astconv.rs/near/207514057" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Arthur Cohen <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Split.20up.20astconv.2Ers.html#207514057">(Aug 20 2020 at 11:56)</a>:</h4>
<p>Sorry for the ping in case it wasn't the right place</p>



<a name="207514191"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Split%20up%20astconv.rs/near/207514191" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> oli <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Split.20up.20astconv.2Ers.html#207514191">(Aug 20 2020 at 11:58)</a>:</h4>
<p>no this is perfect</p>



<a name="207514202"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Split%20up%20astconv.rs/near/207514202" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> oli <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Split.20up.20astconv.2Ers.html#207514202">(Aug 20 2020 at 11:59)</a>:</h4>
<p>yea, what I mean is you moved <code>astconv-rs</code> to <code>astconv/mod.rs</code>, but that move is not necessary</p>



<a name="207514218"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Split%20up%20astconv.rs/near/207514218" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> oli <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Split.20up.20astconv.2Ers.html#207514218">(Aug 20 2020 at 11:59)</a>:</h4>
<p>(it's fine though, just mentioning that it's not necessary)</p>



<a name="207514237"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Split%20up%20astconv.rs/near/207514237" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> oli <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Split.20up.20astconv.2Ers.html#207514237">(Aug 20 2020 at 11:59)</a>:</h4>
<p>the <code>mod astconv</code> will pick up either file if it exists</p>



<a name="207514333"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Split%20up%20astconv.rs/near/207514333" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Arthur Cohen <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Split.20up.20astconv.2Ers.html#207514333">(Aug 20 2020 at 12:00)</a>:</h4>
<p>Oh okay! I thought moving it was better simply for the use of different traits. As you pointed out in your second comment, it's better to just add an <code>impl dyn AstConv</code> in another module than create a second, unnecessary trait. But I moved it to a new module to avoid other files having to do</p>
<div class="codehilite"><pre><span></span><code><span class="k">use</span><span class="w"> </span><span class="n">generic_astconv</span>::<span class="n">AstConvGeneric</span><span class="p">;</span><span class="w"></span>
<span class="k">use</span><span class="w"> </span><span class="n">astconv</span>::<span class="n">AstConv</span><span class="p">;</span><span class="w"></span>
</code></pre></div>


<p>or something along those lines</p>



<a name="207514582"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Split%20up%20astconv.rs/near/207514582" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Arthur Cohen <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Split.20up.20astconv.2Ers.html#207514582">(Aug 20 2020 at 12:04)</a>:</h4>
<p>I'm gonna put the errors in their own file now, since being able to add <code>impl dyn AstConv</code> in other modules unlocks everything</p>



<a name="207514584"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Split%20up%20astconv.rs/near/207514584" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> oli <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Split.20up.20astconv.2Ers.html#207514584">(Aug 20 2020 at 12:04)</a>:</h4>
<p>What I'm trying to say is that your changes will <em>still</em> work, it's just the place where the file lives on the file system that differs</p>



<a name="207514593"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Split%20up%20astconv.rs/near/207514593" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> oli <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Split.20up.20astconv.2Ers.html#207514593">(Aug 20 2020 at 12:04)</a>:</h4>
<p>so you can have a submodule, but still use <code>astconv.rs</code> instead of <code>mod.rs</code></p>



<a name="207514608"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Split%20up%20astconv.rs/near/207514608" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> oli <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Split.20up.20astconv.2Ers.html#207514608">(Aug 20 2020 at 12:04)</a>:</h4>
<p>you'd have <code>astconv.rs</code> and <code>astconv/generic.rs</code> instead of <code>astconv/mod.rs</code> and <code>astconv/generic.rs</code></p>



<a name="207514619"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Split%20up%20astconv.rs/near/207514619" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> oli <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Split.20up.20astconv.2Ers.html#207514619">(Aug 20 2020 at 12:05)</a>:</h4>
<p>but the actual module structure would be the same</p>



<a name="207514627"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Split%20up%20astconv.rs/near/207514627" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Arthur Cohen <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Split.20up.20astconv.2Ers.html#207514627">(Aug 20 2020 at 12:05)</a>:</h4>
<p>Oooooooooh okay</p>



<a name="207514632"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Split%20up%20astconv.rs/near/207514632" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Arthur Cohen <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Split.20up.20astconv.2Ers.html#207514632">(Aug 20 2020 at 12:05)</a>:</h4>
<p>I didn't know about that</p>



<a name="207514635"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Split%20up%20astconv.rs/near/207514635" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Arthur Cohen <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Split.20up.20astconv.2Ers.html#207514635">(Aug 20 2020 at 12:05)</a>:</h4>
<p>Thanks a lot</p>



<a name="207514657"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Split%20up%20astconv.rs/near/207514657" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Arthur Cohen <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Split.20up.20astconv.2Ers.html#207514657">(Aug 20 2020 at 12:05)</a>:</h4>
<p>Sorry for the misunderstanding</p>



<a name="207514727"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Split%20up%20astconv.rs/near/207514727" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Arthur Cohen <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Split.20up.20astconv.2Ers.html#207514727">(Aug 20 2020 at 12:06)</a>:</h4>
<p>If you'd rather have <code>astconv.rs</code> and then <code>astconv/generics.rs</code> let me know, I'll be happy to order it that way</p>



<a name="207514773"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Split%20up%20astconv.rs/near/207514773" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> oli <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Split.20up.20astconv.2Ers.html#207514773">(Aug 20 2020 at 12:07)</a>:</h4>
<p>nah, I think we mostly use <code>mod.rs</code> in rustc, so this change makes sense</p>



<a name="207516295"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Split%20up%20astconv.rs/near/207516295" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Arthur Cohen <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Split.20up.20astconv.2Ers.html#207516295">(Aug 20 2020 at 12:29)</a>:</h4>
<p>Thanks for the review <span class="user-mention" data-user-id="124288">@oli</span> . I added those changes and split up the <code>complain_</code> functions into their own submodule as well</p>



<a name="207516473"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/131828-t-compiler/topic/Split%20up%20astconv.rs/near/207516473" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Arthur Cohen <a href="https://rust-lang.github.io/zulip_archive/stream/131828-t-compiler/topic/Split.20up.20astconv.2Ers.html#207516473">(Aug 20 2020 at 12:31)</a>:</h4>
<p>(And for the precisions)</p>



<hr><p>Last updated: Aug 07 2021 at 22:04 UTC</p>
</html>